Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Updated the Data Source Tags structure #1706

Merged
merged 5 commits into from
Oct 11, 2017
Merged

Updated the Data Source Tags structure #1706

merged 5 commits into from
Oct 11, 2017

Conversation

Ninir
Copy link
Contributor

@Ninir Ninir commented Sep 20, 2017 •
edited
Loading

Issues & Context

Fixes #853
Fixes #1423

Tests

AWS Instance

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSInstanceDataSource_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSInstanceDataSource_ -timeout 120m
=== RUN   TestAccAWSInstanceDataSource_basic
--- PASS: TestAccAWSInstanceDataSource_basic (171.58s)
=== RUN   TestAccAWSInstanceDataSource_tags
--- PASS: TestAccAWSInstanceDataSource_tags (172.30s)
=== RUN   TestAccAWSInstanceDataSource_AzUserData
--- PASS: TestAccAWSInstanceDataSource_AzUserData (171.20s)
=== RUN   TestAccAWSInstanceDataSource_gp2IopsDevice
--- PASS: TestAccAWSInstanceDataSource_gp2IopsDevice (150.83s)
=== RUN   TestAccAWSInstanceDataSource_blockDevices
--- PASS: TestAccAWSInstanceDataSource_blockDevices (157.89s)
=== RUN   TestAccAWSInstanceDataSource_rootInstanceStore
--- PASS: TestAccAWSInstanceDataSource_rootInstanceStore (145.93s)
=== RUN   TestAccAWSInstanceDataSource_privateIP
--- PASS: TestAccAWSInstanceDataSource_privateIP (200.84s)
=== RUN   TestAccAWSInstanceDataSource_keyPair
--- PASS: TestAccAWSInstanceDataSource_keyPair (126.20s)
=== RUN   TestAccAWSInstanceDataSource_VPC
--- PASS: TestAccAWSInstanceDataSource_VPC (203.34s)
=== RUN   TestAccAWSInstanceDataSource_SecurityGroups
--- PASS: TestAccAWSInstanceDataSource_SecurityGroups (158.47s)
=== RUN   TestAccAWSInstanceDataSource_VPCSecurityGroups
--- PASS: TestAccAWSInstanceDataSource_VPCSecurityGroups (222.70s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1881.28s

AWS AMI

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSAmiDataSource_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSAmiDataSource_ -timeout 120m
=== RUN   TestAccAWSAmiDataSource_natInstance
--- PASS: TestAccAWSAmiDataSource_natInstance (38.85s)
=== RUN   TestAccAWSAmiDataSource_windowsInstance
--- PASS: TestAccAWSAmiDataSource_windowsInstance (41.52s)
=== RUN   TestAccAWSAmiDataSource_instanceStore
--- PASS: TestAccAWSAmiDataSource_instanceStore (37.32s)
=== RUN   TestAccAWSAmiDataSource_owners
--- PASS: TestAccAWSAmiDataSource_owners (68.84s)
=== RUN   TestAccAWSAmiDataSource_ownersEmpty
--- PASS: TestAccAWSAmiDataSource_ownersEmpty (126.56s)
=== RUN   TestAccAWSAmiDataSource_localNameFilter
--- PASS: TestAccAWSAmiDataSource_localNameFilter (62.73s)
PASS
ok	github.com/terraform-providers/terraform-provider-aws/aws	376.069s

EBS Snapshot

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSEbsSnapshotDataSource'                                             
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSEbsSnapshotDataSource -timeout 120m
=== RUN   TestAccAWSEbsSnapshotDataSource_basic
--- PASS: TestAccAWSEbsSnapshotDataSource_basic (81.23s)
=== RUN   TestAccAWSEbsSnapshotDataSource_multipleFilters
--- PASS: TestAccAWSEbsSnapshotDataSource_multipleFilters (103.58s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	184.850s

EBS Volume

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSEbsVolumeDataSource_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSEbsVolumeDataSource_ -timeout 120m
=== RUN   TestAccAWSEbsVolumeDataSource_basic
--- PASS: TestAccAWSEbsVolumeDataSource_basic (58.11s)
=== RUN   TestAccAWSEbsVolumeDataSource_multipleFilters
--- PASS: TestAccAWSEbsVolumeDataSource_multipleFilters (54.35s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	112.494s

@Ninir Ninir added the enhancement Requests to existing resources that expand the functionality or scope. label Sep 20, 2017
@Ninir
Copy link
Contributor Author

Ninir commented Sep 20, 2017

Hey @apparentlymart

Since we talked about this in #1423, I would like to know if we really require a migration there (which can be done), or if, as it does not work at all right now, we don't need it?

Will continue to push updates for other data sources.

Thanks!

@Ninir Ninir changed the title Fixed AWS Instance Data Source tags exposure Updated the Data Source Tags structure Sep 20, 2017
@Ninir Ninir changed the title Updated the Data Source Tags structure [WIP] Updated the Data Source Tags structure Sep 20, 2017
Ninir
Ninir commented Sep 20, 2017
View reviewed changes
aws/data_source_aws_ami_ids.go
@@ -34,7 +34,6 @@ func dataSourceAwsAmiIds() *schema.Resource {
ForceNew: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"tags": dataSourceTagsSchema(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not even used in this Data Source

Ninir
Ninir commented Sep 20, 2017
View reviewed changes
aws/data_source_aws_ebs_snapshot_ids.go
@@ -26,7 +26,6 @@ func dataSourceAwsEbsSnapshotIds() *schema.Resource {
ForceNew: true,
Elem: &schema.Schema{Type: schema.TypeString},
},
"tags": dataSourceTagsSchema(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not even used in this Data Source

@Ninir Ninir changed the title [WIP] Updated the Data Source Tags structure Updated the Data Source Tags structure Sep 20, 2017
@Ninir Ninir mentioned this pull request Sep 27, 2017
Closed
@Ninir
Copy link
Contributor Author

Ninir commented Oct 9, 2017

@apparentlymart @radeksimko Letting you the final review :)
As it is not working at the moment, not sure this would need a migration, tell me if so.

Thanks! 👍

@radeksimko radeksimko self-assigned this Oct 11, 2017
radeksimko
radeksimko approved these changes Oct 11, 2017
View reviewed changes
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT data sources don't need migrations in general, because the data is always pulled from scratch as part of every plan/apply.

The reason we usually need a migration for a resource is to avoid no-op diffs (the exact same data, just differently structured) or avoid crashes caused by reading old state. Data sources practically never read old state or generally never compare state and produce diffs.

Also terraform apply -refresh=false is no-op for data sources, so even if it's referenced anywhere, you'll just get the old state, but the Read() method won't get executed.

@Ninir
Copy link
Contributor Author

Ninir commented Oct 11, 2017

Thanks for the info there @radeksimko :)

@Ninir Ninir merged commit d825908 into hashicorp:master Oct 11, 2017
@Ninir Ninir deleted the b-datasources-tags branch October 11, 2017 17:15
@ghost
Copy link

ghost commented Apr 10, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@radeksimko radeksimko radeksimko approved these changes

Assignees

@radeksimko radeksimko

Labels
enhancement Requests to existing resources that expand the functionality or scope.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Get 'value' of particular tag 'key' of aws resource. Can't use tags attribute data sources
2 participants
@Ninir @radeksimko